Split diff line by '\t' for metadata and path#398
Merged
Conversation
This protects against `.split(None)` which uses consecutive whitespace as a separator to overlook paths where a single space is the filename. For example, in this diff line: line = ':100644 000000 e69de29 0000000000000000000000000000000000000000 D ' The deleted file is a file named ' ' (just one space). It's entirely possible to commit this, remove, and to produce the following output from `git diff`: git diff --name-status <SHA1> <SHA2> D M path/to/another/file.py ... This would cause the initial `.split(None, 5)` to fail as it will count all consecutive whitespace as a separator, disregarding the ' ' (single space) filename.
Member
|
Thanks for the elaborate explanation and the fix! Do you think it's feasible to add a test for this as well, maybe with a fixture? |
Contributor
Author
|
And thanks for the quick response @Byron! :) Sure thing, I was just looking into that after I created this PR actually. I'll take a look at the current tests tonight and will see if I can work something in. I think it should be feasible and definitely worth it. |
This tests the edge case of doing a diff against a single whitespace filename and returns the proper change type. All other normal usage of this diff classmethod should remain unchanged.
Contributor
Author
Member
|
Thanks so much, it looks great! |
Byron
added a commit
that referenced
this pull request
Mar 16, 2016
Split diff line by '\t' for metadata and path
Contributor
Author
|
🍻 :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's this PR do?
This protects against
.split(None), which uses consecutive whitespace as a separator and may overlook paths where a single space is the filename. While having a filename consisting of only a single space is unlikely in practical usage, it is entirely possible.For example, take this diff line:
Notice there is a tab after the
change_typeand a space for the path. The deleted file is a file named ' ' (just one space) at the git root. To replicate this issue, add and commit this file, then delete and commit. This will produce the following output fromgit diff:This would cause the initial
.split(None, 5)to fail as it will count all consecutive whitespace as a separator, disregarding the ' ' (single space) filename.